Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix various issues with installation of CamCHILDMine #17

Open
wants to merge 64 commits into
base: master
Choose a base branch
from
Open

Fix various issues with installation of CamCHILDMine #17

wants to merge 64 commits into from

Conversation

martinburchell
Copy link

In the process of getting the 2022 FAIR TREATMENT (CamCHILDMine) demo working I ended up making these changes. In summary:

  • New GitHub workflow for building the docker images with the default Biotestmine
  • Remove obsolete version from docker compose YAML files
  • Abort build.sh if any part of the script fails unexpectedly
  • Store docker-compose defaults in .env file
  • Reduce some complexity in build.sh when resolving environment variables
  • Use wait-for-it to ensure the various containers are running (bundled in as there is no Alpine package)
  • Do not try to copy malaria-data.tar.gz if it is not present
  • Remove the duplicate call to gradlew cargoDeployRemote. In CamCHILDMine I've implemented the workaround described in Gradle needs to start or deploy a second time to serve webapp intermine#2162 (comment)
  • Use named volumes in the local.docker-compose.yml. I've removed the parsing of the YAML file in mkdatadirs.sh as this seemed like a brittle approach and made this script runnable from anywhere.
  • Sort out various permissions issues.

There was a bit of guesswork involved so I'd appreciate another set of eyes on this.

@martinburchell
Workflow to build docker images
0afd95d
@martinburchell
Ignore Emacs autosave files
edc3cb0
@martinburchell
Stacktrace from gradlew cargoDeployRemote
2ec09d4
@martinburchell
Avoid readonly variables in docker compose script
74f67c5
@martinburchell
Check webapp in docker workflow
2571098
@martinburchell
Split up steps of Docker workflow
af17cb7
@martinburchell
Try waiting a bit before dumping logs
bebb6aa 
They are getting truncated
@martinburchell
Try to get a bit more log output
21ec0cd
@martinburchell
Increase workflow timeout to 15 minutes
a480c03
@martinburchell
Use camCHILDMine in workflow
97a1f9a
@martinburchell
Refactor build script mine name variable
f0355c0
@martinburchell
camCHILDMine requires bash in the builder Docker image
fe667db
@martinburchell
Allow mine data volume to be overridden
bbfa848
@martinburchell
Check the right webapp in the Docker workflow
532562f
@martinburchell
Abort on build script failure
20ea19d
@martinburchell
clone camCHILDMine in workflow
76597f7
@martinburchell
Remove duplicate set -e
6acd919
@martinburchell
Try to abort workflow early if build fails
57b0ff7
@martinburchell
Remove obsolete docker compose versions
e556605
@martinburchell
Set default for FORCE_MINE_BUILD
cf35bcb
@martinburchell
Try docker compose run to get build errors from the builder itself
4ea2b21
@martinburchell
Ignore Silver Searcher .ignore file
dc934cd
@martinburchell
Maybe I got those arguments wrong. Also remove the container
d41faad
@martinburchell
Maybe we don't need detach mode
aa6128e
@martinburchell
More uninitialised variables in the build script
80bddbe
@martinburchell
Checkout cadre-dev branch of camCHILDMine in workflow
ac05e96
@martinburchell
Need to change directory before switching git branch
e7e9409
@martinburchell
Fix message in build script
dffdba7
@martinburchell
Fix up project.xml path
3e3a40d
@martinburchell
Fix path to malaria-data.tar.gz in build script
e003498 
Not the permanent solution. This should be ignored if biotestmine is not being used
@martinburchell
Fix mkdatadirs arguments
3f5ca8e
@martinburchell
Put default Docker settings in .env
b3451ae
@martinburchell
Trigger workflows when .env changes [no ci]
f2dd6c7
@martinburchell
Reminder to mkdir for your mine
622be9e
@martinburchell
Wait for postgres to be available or fail
32ea5dc
@martinburchell
Use wait-for-it instead of wait4x to wait for postgres
c00a625
@martinburchell
Fix permissions problem with Postgres container
6bdb8c1
@martinburchell
Bail out early if there was a problem with Solr, Tomcat or Postgres
682322e
@martinburchell
Run Docker and Postgres containers as root
0640b40 
There are permissions problems if we try to use a different user
@martinburchell
Try PGPASSWORD for postgres and reinstate user for Solr
f050524
@martinburchell
Fix Solr user
7ee825b
@martinburchell
Ensure Solr volume is owned by the Solr user
70420f6 
Don't try to do anything with the Solr UID/GID
@martinburchell
Use docker compose up now the build seems to be working
d2efdfa 
cargoDeployRemote fails if run for a second time
@martinburchell
Move checking the webapp so we dump the docker logs on failure
4156e9b
@martinburchell
Fix camchildmine web app path
b23960b
@martinburchell
Fix Postgres health check
75fd434
@martinburchell
Don't copy malaria-data.tar.gz if it doesn't exist
25106a8
@martinburchell
Move CamCHILDMine workflow to that project
9423d79
@martinburchell
Less noisy output of shell scripts
df8b4fd
@martinburchell
Fix mkdatadirs.sh to run from anywhere
39841d8
@martinburchell
Add note about potential 404 when building the webapp
9ca3a08
@martinburchell
Remove debug code in Dockerfile
1b26a48
@martinburchell
Add licence for wait-for-it
53e9856
@martinburchell
Use separate memory configuration for builder, Solr and Tomcat
c9d9ab5
@martinburchell
Remove debugging from build.sh
fcf7844
@martinburchell martinburchell mentioned this pull request Dec 18, 2024
Open
leoank
leoank reviewed Jan 6, 2025
View reviewed changes
Copy link
Member

@leoank leoank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it look good to me.

.env Show resolved Hide resolved
.github/workflows/biotestmine.yml Show resolved Hide resolved
dockerhub.docker-compose.yml Show resolved Hide resolved
intermine_builder/build.sh Show resolved Hide resolved
@leoank
Copy link
Member

leoank commented Jan 6, 2025

@heralden please have a look as well. This PR introduces some ARGS expected by the intermine_builder dockerfile. Is this going to break any of our existing pipelines?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leoank leoank leoank left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@martinburchell @leoank